add cached columns for event creation and last errors, reducing the number of SQL queries
Andrew Cantino
22 измененных файлов с 101 добавлено и 41 удалено
+ 1
- 0
app/assets/javascripts/application.js.coffee.erb
@@ -102,6 +102,7 @@ $(document).ready -> |
||
| 102 | 102 |
$("#logs .refresh, #logs .clear").hide()
|
| 103 | 103 |
$.post "/agents/#{agentId}/logs/clear", { "_method": "DELETE" }, (html) =>
|
| 104 | 104 |
$("#logs .logs").html html
|
| 105 |
+ $("#show-tabs li a.recent-errors").removeClass 'recent-errors'
|
|
| 105 | 106 |
$("#logs .spinner").stop(true, true).fadeOut ->
|
| 106 | 107 |
$("#logs .refresh, #logs .clear").show()
|
| 107 | 108 |
|
+ 4
- 0
app/assets/stylesheets/application.css.scss.erb
@@ -122,3 +122,7 @@ span.not-applicable:after {
|
||
| 122 | 122 |
margin: 0 10px; |
| 123 | 123 |
} |
| 124 | 124 |
} |
| 125 |
+ |
|
| 126 |
+#show-tabs li a.recent-errors {
|
|
| 127 |
+ font-weight: bold; |
|
| 128 |
+} |
+ 1
- 1
app/controllers/events_controller.rb
@@ -6,7 +6,7 @@ class EventsController < ApplicationController |
||
| 6 | 6 |
@agent = current_user.agents.find(params[:agent]) |
| 7 | 7 |
@events = @agent.events.page(params[:page]) |
| 8 | 8 |
else |
| 9 |
- @events = current_user.events.page(params[:page]) |
|
| 9 |
+ @events = current_user.events.preload(:agent).page(params[:page]) |
|
| 10 | 10 |
end |
| 11 | 11 |
|
| 12 | 12 |
respond_to do |format| |
+ 1
- 1
app/controllers/logs_controller.rb
@@ -7,7 +7,7 @@ class LogsController < ApplicationController |
||
| 7 | 7 |
end |
| 8 | 8 |
|
| 9 | 9 |
def clear |
| 10 |
- @agent.logs.delete_all |
|
| 10 |
+ @agent.delete_logs! |
|
| 11 | 11 |
index |
| 12 | 12 |
end |
| 13 | 13 |
|
+ 16
- 17
app/models/agent.rb
@@ -19,14 +19,6 @@ class Agent < ActiveRecord::Base |
||
| 19 | 19 |
serialize :options, JSONWithIndifferentAccess |
| 20 | 20 |
serialize :memory, JSONWithIndifferentAccess |
| 21 | 21 |
|
| 22 |
- def options=(o) |
|
| 23 |
- self[:options] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 24 |
- end |
|
| 25 |
- |
|
| 26 |
- def memory=(o) |
|
| 27 |
- self[:memory] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 28 |
- end |
|
| 29 |
- |
|
| 30 | 22 |
validates_presence_of :name, :user |
| 31 | 23 |
validate :sources_are_owned |
| 32 | 24 |
validate :validate_schedule |
@@ -42,7 +34,6 @@ class Agent < ActiveRecord::Base |
||
| 42 | 34 |
has_many :events, :dependent => :delete_all, :inverse_of => :agent, :order => "events.id desc" |
| 43 | 35 |
has_one :most_recent_event, :inverse_of => :agent, :class_name => "Event", :order => "events.id desc" |
| 44 | 36 |
has_many :logs, :dependent => :delete_all, :inverse_of => :agent, :class_name => "AgentLog", :order => "agent_logs.id desc" |
| 45 |
- has_one :most_recent_log, :inverse_of => :agent, :class_name => "AgentLog", :order => "agent_logs.id desc" |
|
| 46 | 37 |
has_many :received_events, :through => :sources, :class_name => "Event", :source => :events, :order => "events.id desc" |
| 47 | 38 |
has_many :links_as_source, :dependent => :delete_all, :foreign_key => "source_id", :class_name => "Link", :inverse_of => :source |
| 48 | 39 |
has_many :links_as_receiver, :dependent => :delete_all, :foreign_key => "receiver_id", :class_name => "Link", :inverse_of => :receiver |
@@ -88,13 +79,20 @@ class Agent < ActiveRecord::Base |
||
| 88 | 79 |
# Implement me in your subclass to test for valid options. |
| 89 | 80 |
end |
| 90 | 81 |
|
| 91 |
- def event_created_within(days) |
|
| 92 |
- event = most_recent_event |
|
| 93 |
- event && event.created_at > days.to_i.days.ago && event.payload.present? && event |
|
| 82 |
+ def options=(o) |
|
| 83 |
+ self[:options] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 84 |
+ end |
|
| 85 |
+ |
|
| 86 |
+ def memory=(o) |
|
| 87 |
+ self[:memory] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 88 |
+ end |
|
| 89 |
+ |
|
| 90 |
+ def event_created_within?(days) |
|
| 91 |
+ last_event_at && last_event_at > days.to_i.days.ago |
|
| 94 | 92 |
end |
| 95 | 93 |
|
| 96 | 94 |
def recent_error_logs? |
| 97 |
- most_recent_log.try(:level) == 4 |
|
| 95 |
+ last_event_at && last_error_log_at && last_error_log_at > (last_event_at - 2.minutes) |
|
| 98 | 96 |
end |
| 99 | 97 |
|
| 100 | 98 |
def sources_are_owned |
@@ -134,10 +132,6 @@ class Agent < ActiveRecord::Base |
||
| 134 | 132 |
self.schedule = nil if cannot_be_scheduled? |
| 135 | 133 |
end |
| 136 | 134 |
|
| 137 |
- def last_event_at |
|
| 138 |
- @memoized_last_event_at ||= most_recent_event.try(:created_at) |
|
| 139 |
- end |
|
| 140 |
- |
|
| 141 | 135 |
def default_schedule |
| 142 | 136 |
self.class.default_schedule |
| 143 | 137 |
end |
@@ -172,6 +166,11 @@ class Agent < ActiveRecord::Base |
||
| 172 | 166 |
end |
| 173 | 167 |
end |
| 174 | 168 |
|
| 169 |
+ def delete_logs! |
|
| 170 |
+ logs.delete_all |
|
| 171 |
+ update_column :last_error_log_at, nil |
|
| 172 |
+ end |
|
| 173 |
+ |
|
| 175 | 174 |
def log(message, options = {})
|
| 176 | 175 |
puts "Agent##{id}: #{message}" unless Rails.env.test?
|
| 177 | 176 |
AgentLog.log_for_agent(self, message, options) |
+ 3
- 0
app/models/agent_log.rb
@@ -14,6 +14,9 @@ class AgentLog < ActiveRecord::Base |
||
| 14 | 14 |
oldest_id_to_keep = agent.logs.limit(1).offset(log_length - 1).pluck("agent_logs.id")
|
| 15 | 15 |
agent.logs.where("agent_logs.id < ?", oldest_id_to_keep).delete_all
|
| 16 | 16 |
end |
| 17 |
+ |
|
| 18 |
+ agent.update_column :last_error_log_at, Time.now if log.level >= 4 |
|
| 19 |
+ |
|
| 17 | 20 |
log |
| 18 | 21 |
end |
| 19 | 22 |
|
+ 1
- 1
app/models/agents/adioso_agent.rb
@@ -40,7 +40,7 @@ module Agents |
||
| 40 | 40 |
end |
| 41 | 41 |
|
| 42 | 42 |
def working? |
| 43 |
- event_created_within(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 43 |
+ event_created_within?(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 44 | 44 |
end |
| 45 | 45 |
|
| 46 | 46 |
def validate_options |
+ 1
- 1
app/models/agents/twitter_publish_agent.rb
@@ -26,7 +26,7 @@ module Agents |
||
| 26 | 26 |
end |
| 27 | 27 |
|
| 28 | 28 |
def working? |
| 29 |
- (event = event_created_within(options['expected_update_period_in_days'])) && event.payload['success'] == true && !recent_error_logs? |
|
| 29 |
+ event_created_within?(options['expected_update_period_in_days']) && most_recent_event.payload['success'] == true && !recent_error_logs? |
|
| 30 | 30 |
end |
| 31 | 31 |
|
| 32 | 32 |
def default_options |
+ 1
- 1
app/models/agents/twitter_stream_agent.rb
@@ -62,7 +62,7 @@ module Agents |
||
| 62 | 62 |
end |
| 63 | 63 |
|
| 64 | 64 |
def working? |
| 65 |
- event_created_within(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 65 |
+ event_created_within?(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 66 | 66 |
end |
| 67 | 67 |
|
| 68 | 68 |
def default_options |
+ 1
- 1
app/models/agents/twitter_user_agent.rb
@@ -48,7 +48,7 @@ module Agents |
||
| 48 | 48 |
end |
| 49 | 49 |
|
| 50 | 50 |
def working? |
| 51 |
- event_created_within(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 51 |
+ event_created_within?(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 52 | 52 |
end |
| 53 | 53 |
|
| 54 | 54 |
def default_options |
+ 1
- 1
app/models/agents/user_location_agent.rb
@@ -30,7 +30,7 @@ module Agents |
||
| 30 | 30 |
MD |
| 31 | 31 |
|
| 32 | 32 |
def working? |
| 33 |
- event_created_within(2) && !recent_error_logs? |
|
| 33 |
+ event_created_within?(2) && !recent_error_logs? |
|
| 34 | 34 |
end |
| 35 | 35 |
|
| 36 | 36 |
def default_options |
+ 1
- 1
app/models/agents/weather_agent.rb
@@ -41,7 +41,7 @@ module Agents |
||
| 41 | 41 |
default_schedule "8pm" |
| 42 | 42 |
|
| 43 | 43 |
def working? |
| 44 |
- event_created_within(2) && !recent_error_logs? |
|
| 44 |
+ event_created_within?(2) && !recent_error_logs? |
|
| 45 | 45 |
end |
| 46 | 46 |
|
| 47 | 47 |
def wunderground |
+ 1
- 1
app/models/agents/website_agent.rb
@@ -44,7 +44,7 @@ module Agents |
||
| 44 | 44 |
UNIQUENESS_LOOK_BACK = 30 |
| 45 | 45 |
|
| 46 | 46 |
def working? |
| 47 |
- event_created_within(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 47 |
+ event_created_within?(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 48 | 48 |
end |
| 49 | 49 |
|
| 50 | 50 |
def default_options |
+ 1
- 1
app/models/agents/weibo_publish_agent.rb
@@ -27,7 +27,7 @@ module Agents |
||
| 27 | 27 |
end |
| 28 | 28 |
|
| 29 | 29 |
def working? |
| 30 |
- (event = event_created_within(options['expected_update_period_in_days'])) && event.payload['success'] == true && !recent_error_logs? |
|
| 30 |
+ event_created_within?(options['expected_update_period_in_days']) && most_recent_event.payload['success'] == true && !recent_error_logs? |
|
| 31 | 31 |
end |
| 32 | 32 |
|
| 33 | 33 |
def default_options |
+ 1
- 1
app/models/agents/weibo_user_agent.rb
@@ -77,7 +77,7 @@ module Agents |
||
| 77 | 77 |
end |
| 78 | 78 |
|
| 79 | 79 |
def working? |
| 80 |
- event_created_within(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 80 |
+ event_created_within?(options['expected_update_period_in_days']) && !recent_error_logs? |
|
| 81 | 81 |
end |
| 82 | 82 |
|
| 83 | 83 |
def default_options |
+ 5
- 6
app/models/event.rb
@@ -7,18 +7,17 @@ class Event < ActiveRecord::Base |
||
| 7 | 7 |
|
| 8 | 8 |
serialize :payload, JSONWithIndifferentAccess |
| 9 | 9 |
|
| 10 |
- def payload=(o) |
|
| 11 |
- self[:payload] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 12 |
- end |
|
| 13 |
- |
|
| 14 |
- |
|
| 15 | 10 |
belongs_to :user |
| 16 |
- belongs_to :agent, :counter_cache => true |
|
| 11 |
+ belongs_to :agent, :counter_cache => true, :touch => :last_event_at |
|
| 17 | 12 |
|
| 18 | 13 |
scope :recent, lambda { |timespan = 12.hours.ago|
|
| 19 | 14 |
where("events.created_at > ?", timespan)
|
| 20 | 15 |
} |
| 21 | 16 |
|
| 17 |
+ def payload=(o) |
|
| 18 |
+ self[:payload] = ActiveSupport::HashWithIndifferentAccess.new(o) |
|
| 19 |
+ end |
|
| 20 |
+ |
|
| 22 | 21 |
def reemit! |
| 23 | 22 |
agent.create_event :payload => payload, :lat => lat, :lng => lng |
| 24 | 23 |
end |
+ 1
- 1
app/views/agents/show.html.erb
@@ -11,7 +11,7 @@ |
||
| 11 | 11 |
<li class='disabled'><a><i class='icon-picture'></i> Summary</a></li> |
| 12 | 12 |
<li class='active'><a href="#details" data-toggle="tab"><i class='icon-indent-left'></i> Details</a></li> |
| 13 | 13 |
<% end %> |
| 14 |
- <li><a href="#logs" data-toggle="tab" data-agent-id="<%= @agent.id %>"><i class='icon-list-alt'></i> Logs</a></li> |
|
| 14 |
+ <li><a href="#logs" data-toggle="tab" data-agent-id="<%= @agent.id %>" class='<%= @agent.recent_error_logs? ? 'recent-errors' : '' %>'><i class='icon-list-alt'></i> Logs</a></li> |
|
| 15 | 15 |
|
| 16 | 16 |
<% if @agent.can_create_events? && @agent.events.count > 0 %> |
| 17 | 17 |
<li><%= link_to '<i class="icon-random"></i> Events'.html_safe, events_path(:agent => @agent.to_param) %></li> |
+ 14
- 0
db/migrate/20131227000021_add_cached_dates_to_agent.rb
@@ -0,0 +1,14 @@ |
||
| 1 |
+class AddCachedDatesToAgent < ActiveRecord::Migration |
|
| 2 |
+ def up |
|
| 3 |
+ add_column :agents, :last_event_at, :datetime |
|
| 4 |
+ execute "UPDATE agents SET last_event_at = (SELECT created_at FROM events WHERE events.agent_id = agents.id ORDER BY id DESC LIMIT 1)" |
|
| 5 |
+ |
|
| 6 |
+ add_column :agents, :last_error_log_at, :datetime |
|
| 7 |
+ execute "UPDATE agents SET last_error_log_at = (SELECT created_at FROM agent_logs WHERE agent_logs.agent_id = agents.id AND agent_logs.level >= 4 ORDER BY id DESC LIMIT 1)" |
|
| 8 |
+ end |
|
| 9 |
+ |
|
| 10 |
+ def down |
|
| 11 |
+ remove_column :agents, :last_event_at |
|
| 12 |
+ remove_column :agents, :last_error_log_at |
|
| 13 |
+ end |
|
| 14 |
+end |
+ 3
- 1
spec/controllers/logs_controller_spec.rb
@@ -19,12 +19,14 @@ describe LogsController do |
||
| 19 | 19 |
|
| 20 | 20 |
describe "DELETE clear" do |
| 21 | 21 |
it "deletes all logs for a specific Agent" do |
| 22 |
+ agents(:bob_weather_agent).last_error_log_at = 2.hours.ago |
|
| 22 | 23 |
sign_in users(:bob) |
| 23 | 24 |
lambda {
|
| 24 | 25 |
delete :clear, :agent_id => agents(:bob_weather_agent).id |
| 25 | 26 |
}.should change { AgentLog.count }.by(-1 * agents(:bob_weather_agent).logs.count)
|
| 26 | 27 |
assigns(:logs).length.should == 0 |
| 27 |
- agents(:bob_weather_agent).logs.count.should == 0 |
|
| 28 |
+ agents(:bob_weather_agent).reload.logs.count.should == 0 |
|
| 29 |
+ agents(:bob_weather_agent).last_error_log_at.should be_nil |
|
| 28 | 30 |
end |
| 29 | 31 |
|
| 30 | 32 |
it "only deletes logs for an Agent owned by the current user" do |
+ 8
- 0
spec/models/agent_log_spec.rb
@@ -67,6 +67,14 @@ describe AgentLog do |
||
| 67 | 67 |
agents(:jane_website_agent).logs.order("agent_logs.id desc").first.message.should == "message 6"
|
| 68 | 68 |
agents(:jane_website_agent).logs.order("agent_logs.id desc").last.message.should == "message 3"
|
| 69 | 69 |
end |
| 70 |
+ |
|
| 71 |
+ it "updates Agents' last_error_log_at when an error is logged" do |
|
| 72 |
+ AgentLog.log_for_agent(agents(:jane_website_agent), "some message", :level => 3, :outbound_event => events(:jane_website_agent_event)) |
|
| 73 |
+ agents(:jane_website_agent).reload.last_error_log_at.should be_nil |
|
| 74 |
+ |
|
| 75 |
+ AgentLog.log_for_agent(agents(:jane_website_agent), "some message", :level => 4, :outbound_event => events(:jane_website_agent_event)) |
|
| 76 |
+ agents(:jane_website_agent).reload.last_error_log_at.to_i.should be_within(2).of(Time.now.to_i) |
|
| 77 |
+ end |
|
| 70 | 78 |
end |
| 71 | 79 |
|
| 72 | 80 |
describe "#log_length" do |
+ 26
- 0
spec/models/agent_spec.rb
@@ -279,6 +279,32 @@ describe Agent do |
||
| 279 | 279 |
end |
| 280 | 280 |
end |
| 281 | 281 |
|
| 282 |
+ describe "recent_error_logs?" do |
|
| 283 |
+ it "returns true if last_error_log_at is near last_event_at" do |
|
| 284 |
+ agent = Agent.new |
|
| 285 |
+ |
|
| 286 |
+ agent.last_error_log_at = 10.minutes.ago |
|
| 287 |
+ agent.last_event_at = 10.minutes.ago |
|
| 288 |
+ agent.recent_error_logs?.should be_true |
|
| 289 |
+ |
|
| 290 |
+ agent.last_error_log_at = 11.minutes.ago |
|
| 291 |
+ agent.last_event_at = 10.minutes.ago |
|
| 292 |
+ agent.recent_error_logs?.should be_true |
|
| 293 |
+ |
|
| 294 |
+ agent.last_error_log_at = 5.minutes.ago |
|
| 295 |
+ agent.last_event_at = 10.minutes.ago |
|
| 296 |
+ agent.recent_error_logs?.should be_true |
|
| 297 |
+ |
|
| 298 |
+ agent.last_error_log_at = 15.minutes.ago |
|
| 299 |
+ agent.last_event_at = 10.minutes.ago |
|
| 300 |
+ agent.recent_error_logs?.should be_false |
|
| 301 |
+ |
|
| 302 |
+ agent.last_error_log_at = 2.days.ago |
|
| 303 |
+ agent.last_event_at = 10.minutes.ago |
|
| 304 |
+ agent.recent_error_logs?.should be_false |
|
| 305 |
+ end |
|
| 306 |
+ end |
|
| 307 |
+ |
|
| 282 | 308 |
describe "scopes" do |
| 283 | 309 |
describe "of_type" do |
| 284 | 310 |
it "should accept classes" do |
+ 9
- 5
spec/models/agents/website_agent_spec.rb
@@ -44,18 +44,22 @@ describe Agents::WebsiteAgent do |
||
| 44 | 44 |
|
| 45 | 45 |
describe '#working?' do |
| 46 | 46 |
it 'checks if events have been received within the expected receive period' do |
| 47 |
+ stubbed_time = Time.now |
|
| 48 |
+ stub(Time).now { stubbed_time }
|
|
| 49 |
+ |
|
| 47 | 50 |
@checker.should_not be_working # No events created |
| 48 | 51 |
@checker.check |
| 49 | 52 |
@checker.reload.should be_working # Just created events |
| 50 | 53 |
|
| 51 | 54 |
@checker.error "oh no!" |
| 52 |
- @checker.reload.should_not be_working # The most recent log is an error |
|
| 55 |
+ @checker.reload.should_not be_working # There is a recent error |
|
| 53 | 56 |
|
| 54 |
- @checker.log "ok now" |
|
| 55 |
- @checker.reload.should be_working # The most recent log is no longer an error |
|
| 57 |
+ stubbed_time = 20.minutes.from_now |
|
| 58 |
+ @checker.events.delete_all |
|
| 59 |
+ @checker.check |
|
| 60 |
+ @checker.reload.should be_working # There is a newer event now |
|
| 56 | 61 |
|
| 57 |
- two_days_from_now = 2.days.from_now |
|
| 58 |
- stub(Time).now { two_days_from_now }
|
|
| 62 |
+ stubbed_time = 2.days.from_now |
|
| 59 | 63 |
@checker.reload.should_not be_working # Two days have passed without a new event having been created |
| 60 | 64 |
end |
| 61 | 65 |
end |